Fix PGC edenSurvivalRate overflow issues in CopyForward#24347
Open
LinHu2016 wants to merge 1 commit into
Open
Conversation
1588b42 to
d3082c3
Compare
d3082c3 to
ddd7264
Compare
Contributor
|
@amicic @dmitripivkine : could one of you review please? |
Contributor
|
Whoops! I thought this was opened in OMR for some reason. But ultimately the review would go to Aleks or Dmitri anyway. |
The edenSurvivalRate represents the fraction of Eden regions that survived a PGC (nominally in the range [0.0, 1.0]). An overflow (rate > 1.0) can cause incorrect heap/Eden size adjustments, producing unexpected allocation spikes and, in extreme cases, out-of-memory errors. The issue is not a regression but was exposed by a recent Eden-resize change. Four overflow sources are addressed: 1. Incorrect edenCountBeforeCollect (denominator) During a PGC the heap may expand, triggering an Eden size recalculation. The newly calculated Eden size can differ significantly from the size at collection start, making the survival rate inaccurate. Fix: use copyForwardStats->_edenEvacuateRegionCount as the denominator instead of getCurrentEdenSizeInRegions(env). 2. Semantic mismatch between edenSurvivorCount and edenEvacuateCount In the first few PGCs after a GMP, previous Eden survivors are ADDRESS_ORDERED_MARKED regions with logicalAge == 0. They enter the evacuate set as non-Eden regions, but their surviving objects spill into age-0 survivor regions that are counted as Eden survivors. This can cause _edenSurvivorRegionCount > _edenEvacuateRegionCount. 3. Over-counting of Eden survivor regions (shared fresh survivors) Because Eden and age-0 non-Eden regions share the same compact group, they can fill the same fresh survivor regions, causing _edenSurvivorRegionCount to be over-counted. Fix2/3:Use edenEvacuateBytes(=edenCountBeforeCollect * regionSize) and edenSurvivorBytes(= copyForwardStats->_copyBytesEden) instead of edenSurvivorCount and nonEdenSurvivorCount to prevent Semantic mismatch and Over-counting issues. TODO: To improve the accuracy of edenSurvivalRate, edenEvacuateBytes should exclude the free bytes remaining in evacuated Eden regions (region->getMemoryPool()->getFreeMemoryAndDarkMatterBytes()), as they do not represent evacuated live data. 4. _scanBytesEden double-counting (cap) During abort recovery, objects that were partially copied (forwarding pointer already installed) are scanned again in the source region, double-counting their bytes in _scanBytesEden. this can push edenSurvivorBytes above edenEvacuateBytes. Fix: cap edenSurvivorBytes at edenEvacuateBytes and add an assertion that thisSurvivalRate <= 1.0. Signed-off-by: lhu <linhu@ca.ibm.com>
ddd7264 to
a349903
Compare
Contributor
Author
|
@amicic @dmitripivkine please review the changes, Thanks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The edenSurvivalRate represents the fraction of Eden regions that
survived a PGC (nominally in the range [0.0, 1.0]). An overflow
(rate > 1.0) can cause incorrect heap/Eden size adjustments, producing
unexpected allocation spikes and, in extreme cases, out-of-memory
errors. The issue is not a regression but was exposed by a recent
Eden-resize change.
Four overflow sources are addressed:
Incorrect edenCountBeforeCollect (denominator)
During a PGC the heap may expand, triggering an Eden size
recalculation. The newly calculated Eden size can differ
significantly from the size at collection start, making the survival
rate inaccurate. Fix: use copyForwardStats->_edenEvacuateRegionCount
as the denominator instead of getCurrentEdenSizeInRegions(env).
Semantic mismatch between edenSurvivorCount and edenEvacuateCount
In the first few PGCs after a GMP, previous Eden survivors are
ADDRESS_ORDERED_MARKED regions with logicalAge == 0. They enter the
evacuate set as non-Eden regions, but their surviving objects spill
into age-0 survivor regions that are counted as Eden survivors.
This can cause _edenSurvivorRegionCount > _edenEvacuateRegionCount.
Over-counting of Eden survivor regions (shared fresh survivors)
Because Eden and age-0 non-Eden regions share the same compact group,
they can fill the same fresh survivor regions, causing
_edenSurvivorRegionCount to be over-counted.
Fix2/3:Use edenEvacuateBytes(=edenCountBeforeCollect * regionSize) and
edenSurvivorBytes(= copyForwardStats->_copyBytesEden) instead of
edenSurvivorCount and nonEdenSurvivorCount to prevent Semantic mismatch
and Over-counting issues.
TODO: To improve the accuracy of edenSurvivalRate, edenEvacuateBytes
should exclude the free bytes remaining in evacuated Eden regions
(region->getMemoryPool()->getFreeMemoryAndDarkMatterBytes()),
as they do not represent evacuated live data.
During abort recovery, objects that were partially copied (forwarding
pointer already installed) are scanned again in the source region,
double-counting their bytes in _scanBytesEden.
this can push edenSurvivorBytes above edenEvacuateBytes.
Fix: cap edenSurvivorBytes at edenEvacuateBytes and add an
assertion that thisSurvivalRate <= 1.0.
Signed-off-by: lhu linhu@ca.ibm.com